Replace netdev with if-addrs to fix macOS compilation#16
Replace netdev with if-addrs to fix macOS compilation#16ivonnyssen wants to merge 8 commits intoRReverser:mainfrom
Conversation
netdev's transitive objc2 dependencies define a blanket IntoIterator impl for &Retained<T> that causes infinite trait recursion when the Rust trait solver evaluates serde_ndim's NDim: IntoIterator bound. Replacing netdev with if-addrs eliminates the root cause entirely, since if-addrs depends only on libc (unix) and windows-sys (windows). The main adaptation is a GroupedInterface struct that re-groups if-addrs' per-address entries by interface name to match the per-interface model the discovery code expects. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix clippy::match_wildcard_for_single_variants by using explicit IfAddr variants instead of _ wildcards - Change GroupedInterface.index to Option<u32> to avoid silently using index 0 (kernel picks interface) when if-addrs returns None; multicast operations now skip the interface with a warning instead - Accumulate is_loopback across all addresses on a grouped interface via OR, rather than only checking the first address seen - Replace test DEFAULT_ADDR with UDP socket trick to find the default route interface (matching old netdev::get_default_interface behaviour), ensuring both v4 and v6 test addresses come from the same interface Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR replaces the netdev crate with if-addrs for network interface enumeration to avoid a macOS compilation failure caused by an objc2-related trait recursion overflow, and updates discovery client/server code to use the new grouped interface representation.
Changes:
- Swap dependency from
netdevtoif-addrsto remove the problematic macOS transitive dependency chain. - Introduce
GroupedInterface+get_active_interfaces()that groups per-addressif-addrsentries by interface. - Update discovery client/server code to use
GroupedInterfaceand handle interface-index absence.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml | Replaces netdev dependency with if-addrs. |
| Cargo.lock | Removes netdev/objc2-related transitive crates and adds if-addrs. |
| src/discovery.rs | Adds GroupedInterface + new interface enumeration; updates tests to find “default” interface IP without netdev. |
| src/client/discovery.rs | Updates client discovery to use grouped interface addresses and optional interface index. |
| src/server/discovery.rs | Updates server multicast joining to use GroupedInterface and new interface enumeration API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let Some(index) = intf.index else { | ||
| tracing::warn!("skipping multicast send: no interface index"); | ||
| return Ok(()); |
| let Some(index) = intf.index else { | ||
| tracing::warn!("skipping multicast join: no interface index"); | ||
| return; | ||
| }; | ||
| match socket.join_multicast_v6(&DISCOVERY_ADDR_V6, index) { | ||
| Ok(()) => tracing::trace!("success"), | ||
| Err(err) => tracing::warn!(%err), |
When multiple address entries exist for the same interface, preserve the first available index rather than discarding it. This prevents multicast operations from being skipped when the first address entry has index=None but a later one provides Some. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
RReverser
left a comment
RReverser
left a comment
There was a problem hiding this comment.
This blocks running CI on macOS. The crate does not compile on macOS at all with the current netdev dependency.
If the goal is to fix compilation on macOS, please add a corresponding CI test. Merely running the discovery test on macOS as a separate job should be enough - no need to run the whole nested matrix of tests.
|
|
||
| #[tracing::instrument(level = "debug", ret, skip(socket))] | ||
| fn join_multicast_groups(socket: &UdpSocket, listen_addr: Ipv6Addr) { | ||
| let interfaces = match get_active_interfaces() { |
There was a problem hiding this comment.
We should surface this as an error through Result. Can't do much discovery if we couldn't get a list of interfaces.
ivonnyssen
force-pushed
the
pr/macos-compilation-fix
branch
from
eb06dd0 to
7089003
Compare
Prior CI ran clippy only on ubuntu-latest. The discovery code had
never been exercised in CI, which hid a latent SO_BROADCAST gap and a
test-only panic on runners lacking link-local IPv6.
- Add a discovery-tests matrix across ubuntu-latest, macos-latest,
windows-latest.
- Share the Swatinem/rust-cache@v2 cache per OS via shared-key so the
seven clippy matrix jobs and three discovery-tests jobs reuse one
cache per OS. ~/.cargo/registry is identical across feature sets so
this is a clear win; target/ reuse is partial but still helps.
- Add two CI-friendly tests that don't touch the DEFAULT_ADDR
LazyLock (which requires link-local IPv6 on the default interface,
unavailable on headless runners):
- test_unspecified_v4_only: server on 0.0.0.0 exercises IPv4 subnet
broadcast discovery.
- test_loopback_v6_only: server on ::1 exercises IPv6 server bind,
join_multicast_v6 on the loopback interface, V6 unicast send, and
V6 response handling end-to-end.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Without SO_BROADCAST, sending to IPv4 subnet broadcast addresses (e.g. 127.255.255.255, 192.168.1.255) returns EACCES on Linux and macOS. Windows accepts the sends regardless, which is why the pre-existing tests appeared to work there but not elsewhere. This was latent because the discovery tests never ran in CI. Discovered while adding the discovery-tests matrix: Ubuntu and macOS jobs failed with "Permission denied (os error 13)" on every send_to, producing zero discovered servers. Also simplify the test assertion to use Iterator::any instead of collecting into a Vec just to check is_empty.
Previously, if if_addrs failed to enumerate interfaces, the server logged a warning and bound successfully with no multicast joins — a silently crippled discovery server. Per PR review: can't do useful discovery if we can't list interfaces, so surface it via Result and let the caller decide. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
On Windows, if-addrs reports separate interface indices for V4 and V6 entries of the same adapter (IfIndex vs Ipv6IfIndex in IP_ADAPTER_ADDRESSES), and these can differ. The previous grouping kept whichever index came first, so IPv6 multicast ops (set_multicast_if_v6, join_multicast_v6) could get the wrong index on Windows when the V4 entry was enumerated first. Rename GroupedInterface.index -> ipv6_index and populate it only from V6 entries. The V4 index is not stored because IPv4 discovery uses subnet-directed broadcast, where interface selection happens via the routing table. On Unix, if_nametoindex returns the same value regardless of address family, so restricting to V6 entries is equivalent. CI can't exercise this directly (our runners have V4-only NICs plus loopback with equal V4/V6 indices) but the fix is a straight read of the Windows API contract. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Switch from 10.254.254.254 to 192.0.2.1 (RFC 5737) in the UDP socket trick used to infer the default-route IP. VPNs commonly push routes for RFC1918 prefixes (10/8, 172.16/12, 192.168/16), so a 10.0.0.0/8 target can resolve to the VPN tun interface instead of the true default route. TEST-NET ranges are reserved for documentation and are very rarely matched by a more-specific route. No effect in CI (no VPN) but reduces flakiness for developers running tests on machines with a corporate VPN. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ivonnyssen
force-pushed
the
pr/macos-compilation-fix
branch
from
dfc6dc5 to
7508725
Compare
|
Thanks for the reviews! Pushed updates (force-push to squash 10 exploratory commits into 5 clean ones). Summary: Addressed
Declining (with evidence)
Bonus fix surfaced during investigation
Final commit stack on top of All three OSes green. |
Summary
Replaces the
netdevdependency withif-addrsto fix a trait recursion overflow that prevents compilation on macOS.Problem
netdevtransitively depends onobjc2, which defines a blanketIntoIteratorimpl for&Retained<T>. This causes infinite trait recursion when the Rust trait solver evaluatesserde_ndim'sNDim: IntoIteratorbound during compilation of the camera image array serialization code. This is a known Rust compiler issue (rust-lang/rust#136856).Solution
if-addrsprovides the same network interface enumeration functionality using onlylibc(unix) /windows-sys(windows) — no objc2 dependency chain. This eliminates the root cause entirely.The main adaptation is a
GroupedInterfacestruct that re-groupsif-addrs' per-address entries by interface name, sinceif-addrsreturns one entry per address while the discovery code expects one entry per interface.Motivation
This blocks running CI on macOS. The crate does not compile on macOS at all with the current
netdevdependency.Changes
Cargo.toml: Replacenetdevwithif-addrssrc/discovery.rs: AddGroupedInterfacestruct withget_active_interfaces()function; merge interface index across address entries so multicast isn't skipped when the first entry lacks an indexsrc/client/discovery.rs: Adapt toGroupedInterfaceAPIsrc/server/discovery.rs: Adapt toGroupedInterfaceAPI, improve multi-homed network handlingCargo.lock: UpdatedTest plan
cargo clippypasses (full CI feature matrix verified on Linux)